Assorted bootstrap LLVM refactors (part 3/N) - #161247
Conversation
|
This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. |
|
|
This comment has been minimized.
This comment has been minimized.
953c4f0 to
cf840fa
Compare
There was a problem hiding this comment.
Thanks, r=me after pr ci green
@bors rollup=never note="bootstrap llvm rework just in case"
|
@bors r=jieyouxu |
|
@bors p=6 scheduling |
This comment has been minimized.
This comment has been minimized.
Assorted bootstrap LLVM refactors (part 3/N) Continuation of #160916. This PR removes the `llvm_out` function, and makes it explicit when we need to build the host LLVM. Before, bootstrap used to just arbitrarily run a `llvm-config` binary, even though it might not have been executable on the given host. Now, if we need to execute it, the host LLVM will always be explicitly built. It is possible that there are some cases where this will build LLVM where it didn't before, but that should only happen if you were on a target A, built LLVM for target B, and by sheer luck A could execute code for B (where A != B). In any case, it is now explicit, so if we encounter such situation, we can fix it without depending on implicit assumptions (well, there are still thousands of other assumptions, but you get my point). After this, I'll work on centralizing the sanity checking of downloading LLVM inside the `LlvmFromCi` step, which will allow us to log the exact reason why `download-ci-llvm` might not have been applied, and also enable downloading LLVM from CI for non-host targets. r? jieyouxu
This comment has been minimized.
This comment has been minimized.
|
💔 Test for 55e7b47 failed: CI. Failed job:
|
|
@bors try jobs=dist-aarch64-freebsd |
This comment has been minimized.
This comment has been minimized.
Assorted bootstrap LLVM refactors (part 3/N) try-job: dist-aarch64-freebsd
This comment has been minimized.
This comment has been minimized.
And explicitly depend on building the host LLVM when we need to invoke its `llvm-config` binary.
4b94774 to
c6d2681
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
Assorted bootstrap LLVM refactors (part 3/N) try-job: dist-aarch64-freebsd
This comment has been minimized.
This comment has been minimized.
|
💔 Test for ad9d7cd failed: CI. Failed job:
|
This comment has been minimized.
This comment has been minimized.
|
@bors r- |
|
This pull request was unapproved. |
|
@bors try jobs=dist-aarch64-freebsd |
This comment has been minimized.
This comment has been minimized.
Assorted bootstrap LLVM refactors (part 3/N) try-job: dist-aarch64-freebsd
|
@bors r=jieyouxu |
|
⌛ Testing commit 730b9d8 with merge ac62df9... Workflow: https://github.com/rust-lang/rust/actions/runs/32717790533 |
Assorted bootstrap LLVM refactors (part 3/N) Continuation of #160916. This PR removes the `llvm_out` function, and makes it explicit when we need to build the host LLVM. Before, bootstrap used to just arbitrarily run a `llvm-config` binary, even though it might not have been executable on the given host. Now, if we need to execute it, the host LLVM will always be explicitly built. It is possible that there are some cases where this will build LLVM where it didn't before, but that should only happen if you were on a target A, built LLVM for target B, and by sheer luck A could execute code for B (where A != B). In any case, it is now explicit, so if we encounter such situation, we can fix it without depending on implicit assumptions (well, there are still thousands of other assumptions, but you get my point). After this, I'll work on centralizing the sanity checking of downloading LLVM inside the `LlvmFromCi` step, which will allow us to log the exact reason why `download-ci-llvm` might not have been applied, and also enable downloading LLVM from CI for non-host targets. r? jieyouxu
View all comments
Continuation of #160916.
This PR removes the
llvm_outfunction, and makes it explicit when we need to build the host LLVM.Before, bootstrap used to just arbitrarily run a
llvm-configbinary, even though it might not have been executable on the given host. Now, if we need to execute it, the host LLVM will always be explicitly built. It is possible that there are some cases where this will build LLVM where it didn't before, but that should only happen if you were on a target A, built LLVM for target B, and by sheer luck A could execute code for B (where A != B).In any case, it is now explicit, so if we encounter such situation, we can fix it without depending on implicit assumptions (well, there are still thousands of other assumptions, but you get my point).
After this, I'll work on centralizing the sanity checking of downloading LLVM inside the
LlvmFromCistep, which will allow us to log the exact reason whydownload-ci-llvmmight not have been applied, and also enable downloading LLVM from CI for non-host targets.r? jieyouxu